feat: add `math/base/special/factorial2f` #6736

anandkaranubc · 2025-04-19T23:51:12Z

Progresses #649

Description

What is the purpose of this pull request?

This pull request:

Adds the JS and C implementation of math/base/special/factorial2f.

Related Issues

Does this pull request have any related issues?

This pull request:

progresses [RFC]: Add C implementations to base special math functions (tracking issue) #649

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

Read, understood, and followed the contributing guidelines.

@stdlib-js/reviewers

--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---

--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: passed - task: lint_package_json status: passed - task: lint_repl_help status: passed - task: lint_javascript_src status: passed - task: lint_javascript_cli status: na - task: lint_javascript_examples status: passed - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: passed - task: lint_python status: passed - task: lint_r status: na - task: lint_c_src status: passed - task: lint_c_examples status: passed - task: lint_c_benchmarks status: passed - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: passed - task: lint_typescript_tests status: passed - task: lint_license_headers status: passed ---

--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: passed - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: passed - task: lint_javascript_cli status: na - task: lint_javascript_examples status: passed - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: passed - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---

lib/node_modules/@stdlib/math/base/special/factorial2f/README.md

stdlib-bot · 2025-04-19T23:53:00Z

Coverage Report

Package	Statements	Branches	Functions	Lines
math/base/special/factorial2f	$\color{green}203/203$ $\color{green}+100.00\%$	$\color{green}19/19$ $\color{green}+100.00\%$	$\color{green}2/2$ $\color{green}+100.00\%$	$\color{green}203/203$ $\color{green}+100.00\%$

The above coverage report was generated for the changes in this PR.

--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: passed - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---

lib/node_modules/@stdlib/math/base/special/factorial2f/test/test.native.js

--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: passed - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---

kgryte · 2025-04-20T06:26:22Z

lib/node_modules/@stdlib/math/base/special/factorial2f/src/main.c

+	if ( n < 0 || !stdlib_base_is_integerf( n ) ) {
+		return 0.0f / 0.0f; // NaN
+	}
+	v = n;


v is not necessary. We do that in JS in order to cast the implicit double input argument to int32 using an asm type annotation.

--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: passed - task: lint_package_json status: passed - task: lint_repl_help status: na - task: lint_javascript_src status: passed - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: passed - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---

anandkaranubc · 2025-04-20T08:14:21Z

lib/node_modules/@stdlib/math/base/special/factorial2f/test/fixtures/python/runner.py

+    python> gen(x, './data.json')
+    ```
+    """
+    y = factorial2(x, exact=False)


Suggested change

y = factorial2(x, exact=False)

y = factorial2(x, exact=False).astype(np.float32)

However, this change still doesn't affect the return values of the test fixtures.

--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---

lib/node_modules/@stdlib/math/base/special/factorial2f/test/test.native.js

anandkaranubc · 2025-04-20T08:31:50Z

@kgryte Sorry if my recent comments cause any confusion. Just wanted to summarize everything I found so far.

kgryte · 2025-04-20T08:34:23Z

lib/node_modules/@stdlib/math/base/special/factorial2f/src/main.c

+	}
+	out = 1.0f;
+	for ( i = n; i >= last; i -= 2 ) {
+		out *= i;


In C, this is equivalent to out = out * (float)i;.

lib/node_modules/@stdlib/math/base/special/factorial2f/lib/main.js

--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: passed - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---

--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: na - task: lint_python status: passed - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---

--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: passed - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: passed - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---

lib/node_modules/@stdlib/math/base/special/factorial2f/README.md

Signed-off-by: Athan <[email protected]>

lib/node_modules/@stdlib/math/base/special/factorial2f/lib/index.js

Signed-off-by: Athan <[email protected]>

lib/node_modules/@stdlib/math/base/special/factorial2f/package.json

Signed-off-by: Athan <[email protected]>

lib/node_modules/@stdlib/math/base/special/factorial2f/lib/main.js

Signed-off-by: Athan <[email protected]>

kgryte · 2025-04-23T02:01:08Z

lib/node_modules/@stdlib/math/base/special/factorial2f/src/main.c

+	if ( stdlib_base_is_nanf( n ) ) {
+		return 0.0f / 0.0f; // NaN
+	}


Suggested change

if ( stdlib_base_is_nanf( n ) ) {

return 0.0f / 0.0f; // NaN

}

This branch is not possible. n is int32_t.

kgryte · 2025-04-23T02:01:54Z

lib/node_modules/@stdlib/math/base/special/factorial2f/src/main.c

+	if ( n > STDLIB_CONSTANT_FLOAT32_MAX_NTH_DOUBLE_FACTORIAL ) {
+		return STDLIB_CONSTANT_FLOAT32_PINF;
+	}
+	if ( n < 0 || !stdlib_base_is_integerf( n ) ) {


Suggested change

if ( n < 0 || !stdlib_base_is_integerf( n ) ) {

if ( n < 0 ) {

n is int32_t. There is never an instance in which n is not an integer. This means that the manifest.json should be updated, along with the list of includes above.

kgryte · 2025-04-23T02:02:56Z

lib/node_modules/@stdlib/math/base/special/factorial2f/src/main.c

+	if ( n == 0 || n == 1 ) {
+		return 1.0f;
+	}
+	if ( stdlib_base_is_evenf( n ) ) {


Don't use is_evenf here. We know that n is an integer. Use n&1 == 0. Meaning, check the last bit. Manifest and list of includes should be updated accordingly.

Actually, use int32-is-even: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/math/base/assert/int32-is-even

kgryte · 2025-04-23T02:04:51Z

lib/node_modules/@stdlib/math/base/special/factorial2f/test/test.js

+			t.strictEqual( v, expected[ i ], 'returns expected value' );
+		} else {
+			delta = absf( v - expected[i] );
+			tol = 2.4 * EPS * absf( expected[i] );


We still need approximate equality? Is that because the fixtures are generated at higher precision?

kgryte · 2025-04-23T02:06:10Z

lib/node_modules/@stdlib/math/base/special/factorial2f/test/fixtures/python/runner.py

+
+def main():
+    """Generate fixture data."""
+    x = np.arange(1, 57, dtype=np.int32)


You may be able to adjust the precision here by using np.float32. np.int32 is likely to cause factorial2 to be evaluated in double-precision. NumPy is typically good about preserving float precision.

Thanks @kgryte for the review, and sorry for the mistakes.

Also, I tried using np.float32, which requires the factorial2 function to be called with the additional parameter extend='complex'. As the documentation mentions, this changes the values of the double factorial for even integers by reducing them by a factor of sqrt(2/π). But even if we ignore that, the results for odd integers don't match perfectly either.

After a bit of head-twisting and surfing through the source code, I found that SciPy uses the gamma function to approximate the double factorial, rather than the straightforward iterative method we typically use. They have a wrapper function that generalizes the implementation for all types of factorials.

https://github.com/scipy/scipy/blob/0f1fd4a7268b813fa2b844ca6038e4dfdf90084a/scipy/special/_basic.py#L2992-L2997

~~I could just generate the expected values locally using MATLAB.~~

Update: No luck with that :(

No worries. Just add a note to the tests explaining why we are using approximate equality.

--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: na - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: passed - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---

--- type: pre_commit_static_analysis_report description: Results of running static analysis checks when committing changes. report: - task: lint_filenames status: passed - task: lint_editorconfig status: passed - task: lint_markdown status: na - task: lint_package_json status: na - task: lint_repl_help status: na - task: lint_javascript_src status: na - task: lint_javascript_cli status: na - task: lint_javascript_examples status: na - task: lint_javascript_tests status: passed - task: lint_javascript_benchmarks status: na - task: lint_python status: na - task: lint_r status: na - task: lint_c_src status: na - task: lint_c_examples status: na - task: lint_c_benchmarks status: na - task: lint_c_tests_fixtures status: na - task: lint_shell status: na - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---

anandkaranubc added 6 commits March 29, 2025 14:31

feat: temp commit

9e94d1f

Merge branch 'develop' into feat/factorial2f

6dc82d7

stdlib-bot added Math Issue or pull request specific to math functionality. Needs Review A pull request which needs code review. labels Apr 19, 2025

anandkaranubc commented Apr 19, 2025

View reviewed changes

lib/node_modules/@stdlib/math/base/special/factorial2f/README.md Show resolved Hide resolved

anandkaranubc commented Apr 19, 2025

View reviewed changes

lib/node_modules/@stdlib/math/base/special/factorial2f/test/test.native.js Show resolved Hide resolved

anandkaranubc added Feature Issue or pull request for adding a new feature. JavaScript Issue involves or relates to JavaScript. C Issue involves or relates to C. labels Apr 20, 2025

kgryte reviewed Apr 20, 2025

View reviewed changes

anandkaranubc commented Apr 20, 2025

View reviewed changes

lib/node_modules/@stdlib/math/base/special/factorial2f/test/test.native.js Show resolved Hide resolved

kgryte reviewed Apr 20, 2025

View reviewed changes

lib/node_modules/@stdlib/math/base/special/factorial2f/lib/main.js Outdated Show resolved Hide resolved

anandkaranubc added 2 commits April 20, 2025 01:47

anandkaranubc marked this pull request as draft April 21, 2025 15:34

stdlib-bot removed the Needs Review A pull request which needs code review. label Apr 21, 2025

anandkaranubc added 2 commits April 22, 2025 15:49

Merge branch 'develop' into feat/factorial2f

8996360

anandkaranubc marked this pull request as ready for review April 22, 2025 22:54

stdlib-bot added the Needs Review A pull request which needs code review. label Apr 22, 2025